[SUR-488] [ForceUI] Add reverse position support for the bordered variant radio-button component#471
[SUR-488] [ForceUI] Add reverse position support for the bordered variant radio-button component#471jaieds wants to merge 1 commit into
Conversation
- Anchor the selection control at left-3 (mirroring the default right-3 inset) instead of left-0 + right-3, which stretched the control wrapper across the card and pinned it flush against the border. - Mirror the horizontal padding when reversed (pl-12 radio / pl-16 switch) instead of the hardcoded pr-12, and drop the ml-4/ml-10 label margin hacks. - Wrap label heading/description with overflow-wrap:anywhere so long unbroken words no longer overflow the bordered card in narrow containers. - Add missing 1.5px borderWidth to the theme scale so the radio and checkbox !border-1.5 class resolves (previously fell back to the 3px browser default). - Add bordered reverse-position stories for radio and switch variants. Closes brainstormforce/surerank#1905 (SUR-488)
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -157,6 +157,81 @@ RadioWithBorderSmallSize.args = { | |||
| size: 'sm', | |||
There was a problem hiding this comment.
What: Consider adding comments to describe the purpose of new story components such as RadioWithBorderReversePosition and SwitchWithBorderReversePosition. It's not immediately clear what these stories are demonstrating or why they are useful.
Why: Adding comments will help other developers (or future you) understand the intended use and functionality of these stories, improving maintainability and clarity.
How: For instance, add comments like // Story demonstrating the reverse position for bordered RadioButton above each story definition.
| value={ `option${ num }` } | ||
| label={ { | ||
| heading: `Option ${ num }`, | ||
| } } |
There was a problem hiding this comment.
What: The new RadioButton stories have added complexity with monitoring state management. Ensure that the initial state is handled carefully to avoid unintentional mutations or leaks in state management.
Why: Improper handling of component state can lead to bugs that are difficult to trace and can affect usability and performance.
How: Consider using functional updates to set state or utilizing React's useReducer for complex state logic to prevent inadvertent state bugs.
| ) => { | ||
| const [ value, setValue ] = useState( args.value || args.defaultValue ); | ||
|
|
||
| return ( |
There was a problem hiding this comment.
What: In the new templates, you're repeating the setup code for RadioButton and Switch components. Consider creating a reusable function to encapsulate this logic.
Why: Duplicating code can lead to maintenance issues since a change in one instance would require a change in all. It decreases readability and increases potential for bugs.
How: You can create a helper function that returns the RadioButton.Group setup and just modify it as necessary for different scenarios.
| @@ -358,7 +357,7 @@ export const RadioButtonComponent = ( | |||
| > | |||
| <p | |||
There was a problem hiding this comment.
What: Using [overflow-wrap:anywhere] in your class names may introduce non-standard handling of overflow that could affect performance in rare scenarios or introduce browser inconsistencies.
Why: Maintaining a balance between CSS standards and performance is crucial. Overly broad or non-standard CSS can lead to unexpected behavior across different browsers, which can also potentially influence performance.
How: Consider using a more standard CSS approach for overflowing text, such as setting overflow-wrap: break-word; in a dedicated CSS class or style instead.
| @@ -405,6 +404,13 @@ export const RadioButtonComponent = ( | |||
| } | |||
There was a problem hiding this comment.
What: The conditional assignment of controlSpacingClass can be optimized for readability and maintainability.
Why: Improving conditional logic for class assignment can enhance code legibility and decrease potential for errors when toggling classes, especially as components evolve.
How: Consider using the ternary operator directly in the class list like this: const controlSpacingClass = reversePosition ? (useSwitch ? 'pl-16' : 'pl-12') : 'pr-12'; This reduces the need for an additional block of code and keeps related logic together.
jaieds
changed the title
mohitsbsftester
left a comment
mohitsbsftester
left a comment
There was a problem hiding this comment.
There is a cross-PR regression with the Tailwind v4 migration in #470 that we should handle before both land.
This PR fixes border-1.5 by adding 1.5: '1.5px' only to src/theme/default-config.js. #470 moves Force UI v2 consumers to the CSS-first theme.css path and keeps the component class as border-1.5!, but its new src/theme/theme.css has no --border-width-1_5 / escaped 1.5 border-width token or equivalent border-1.5 utility.
In Tailwind v4 the border width resolver only accepts a themed --border-width value here or a positive integer bare value. 1.5 is not a positive integer, so CSS-first v2 consumers will again generate no border-1.5 rule even though this PR fixes the legacy JS-config path.
Please carry the 1.5px border-width parity into #470's CSS theme/utility path as well, and validate the merged #470 + #471 state. Otherwise the radio/checkbox 1.5px fix regresses immediately for the new recommended integration.
4 participants
What
Adds working
reversePositionsupport for the bordered (borderOn) RadioButton variant: the radio/switch control renders inset on the left edge inside the border with mirrored horizontal padding, label on the right.Closes brainstormforce/surerank#1905 (SUR-488)
Changes
radio-button.tsxleft-3 ml-0.5when reversed (previously got bothleft-0andright-3, stretching across the card and sitting flush against the border).pl-12(radio) /pl-16(switch) replaces the hardcodedpr-12; removed the oldml-4/ml-10label margin hacks.overflow-wrap: anywhereso long unbroken words no longer overflow the card border in narrow containers.theme/default-config.js— added1.5: '1.5px'to theborderWidthscale. The!border-1.5class used by radio and checkbox generated no CSS, so both rendered with the 3px browser-default border; they now render at the intended 1.5px.radio-button.stories.tsx— newRadioWithBorderReversePositionandSwitchWithBorderReversePositionstories.changelog.txt— entry added.Testing
tsc -b, ESLint (./src), Stylelint: clean